Skip to content

fix: table comparsion query for date type column#4649

Open
Aries0d0f wants to merge 4 commits into
simstudioai:stagingfrom
Aries0d0f:fix/table-comparsion-query-for-date-type-column
Open

fix: table comparsion query for date type column#4649
Aries0d0f wants to merge 4 commits into
simstudioai:stagingfrom
Aries0d0f:fix/table-comparsion-query-for-date-type-column

Conversation

@Aries0d0f
Copy link
Copy Markdown

@Aries0d0f Aries0d0f commented May 18, 2026

Summary

Fix broken date/timestamp comparisons in the table filter query builder.

buildComparisonClause always cast comparison values with ::numeric, so filters like { createdAt: { $gte: '2024-01-01' } } on a date-typed column produced invalid SQL and returned no results. The fix threads column type metadata through the filter pipeline so date columns use ::timestamp instead.

Fixes #(issue)

Type of Change

  • Bug fix

Testing

Unit tests in apps/sim/lib/table/__tests__/sql.test.ts under the new date column type suite:

  • $gt, $gte, $lt, $lte with a date column — asserts ::timestamp is used and ::numeric is absent
  • Combined date range ($gte + $lte on the same field)
  • Date column inside $and — asserts column type propagates through logical operators
  • Regression guard: number column still produces ::numeric, not ::timestamp

Also verified that this fix works by manually build image.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Screenshots/Videos

N/A — no UI changes.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 18, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 18, 2026 0:49am

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 18, 2026

PR Summary

Medium Risk
Touches shared filter SQL generation used by query/update/delete paths, so an incorrect cast or missing column metadata could change which rows are matched. Changes are scoped and covered by new unit tests for date casting behavior.

Overview
Fixes table row filtering for date-typed columns by threading column metadata into buildFilterClause and emitting ::timestamp casts for range operators ($gt/$gte/$lt/$lte) when the target column is type: 'date' (keeping ::numeric for number fields).

Updates API routes and service/tool call sites to pass schema.columns/table into filter building (including bulk deleteRowsByFilter), and expands SQL builder unit tests to assert the new casting behavior and guard numeric comparisons.

Reviewed by Cursor Bugbot for commit daa1047. Bugbot is set up for automated code reviews on this repo. Configure here.

@Aries0d0f Aries0d0f changed the title Fix/table comparsion query for date type column fix: table comparsion query for date type column May 18, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 18, 2026

Greptile Summary

This PR fixes incorrect SQL casts for date column comparisons in the table filter query builder by threading column type metadata through buildFilterClausebuildFieldConditionbuildComparisonClause, emitting ::timestamp instead of ::numeric when the column type is 'date'.

  • sql.ts: buildFilterClause gains an optional columns?: ColumnDefinition[] parameter; a columnTypeMap is derived from it and passed down to buildComparisonClause, which now uses ::timestamp for date columns and ::numeric otherwise.
  • types.ts: $gt/$gte/$lt/$lte in ConditionOperators are widened from number to number | string to allow date strings.
  • sql.test.ts: New date column type suite verifies ::timestamp is used for all four range operators, a combined range, an $and-nested filter, and a numeric regression guard.

Confidence Score: 3/5

The SQL builder change is logically correct, but the fix doesn't reach production because no call site passes the new columns argument — date filters will continue to fail at runtime.

Every real call site — both route handlers and all three service.ts call sites — omits the new columns argument. In both route handlers table.schema.columns is already in scope; in updateRowsByFilter the table parameter is received but its columns aren't forwarded. The unit tests pass because they explicitly supply columns, masking the gap. Merging as-is leaves the reported bug unfixed in all API-facing paths.

apps/sim/lib/table/service.ts (lines 1438, 1821, 2132) and both route handler files need the columns argument wired in before this fix has any effect in production.

Important Files Changed

Filename Overview
apps/sim/lib/table/sql.ts Adds optional columns parameter to buildFilterClause and propagates column type through buildFieldCondition and buildComparisonClause to emit ::timestamp instead of ::numeric for date columns. The SQL builder change itself is correct, but none of the production callers have been updated to pass columns, so the fix is inert in production.
apps/sim/lib/table/types.ts Widens $gt/$gte/$lt/$lte in ConditionOperators from number to `number
apps/sim/lib/table/tests/sql.test.ts Adds a date column type test suite covering all four comparison operators, a range filter, $and propagation, and a numeric regression guard. Coverage is good but the getRawSqlString helper relies on undocumented drizzle internals that could silently break.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["buildFilterClause(filter, tableName, columns?)"] --> B["Build columnTypeMap from columns"]
    B --> C{Field type?}
    C -->|"$or / $and"| D["buildLogicalClause(..., columns)"]
    D --> A
    C -->|"regular field"| E["buildFieldCondition(..., columnTypeMap.get(field))"]
    E --> F{Operator}
    F -->|"$gt/$gte/$lt/$lte"| G["buildComparisonClause(..., columnType?)"]
    G --> H{columnType === 'date'?}
    H -->|Yes| I["(data->>'field')::timestamp OP value::timestamp"]
    H -->|No| J["(data->>'field')::numeric OP value"]
Loading

Reviews (1): Last reviewed commit: "test: Update test for verify date column..." | Re-trigger Greptile

Comment on lines +17 to +30
function getRawSqlString(sqlObj: SQL): string {
const obj = sqlObj as unknown as Record<string, unknown>
if (Array.isArray(obj['queryChunks'])) {
return (obj['queryChunks'] as unknown[])
.map((chunk) => {
if (typeof chunk === 'string') return chunk
const raw = chunk as { value?: string[] }
return raw.value?.join('') ?? ''
})
.join('')
}
return JSON.stringify(sqlObj)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 getRawSqlString relies on undocumented drizzle internals

The helper reads queryChunks from the drizzle SQL object, which is not part of drizzle-orm's public API. The inner shape ({ value?: string[] }) also doesn't match the actual exported StringChunk / Param types in drizzle — a minor drizzle version bump could silently change the internal structure, causing these assertions to always pass (fallback to JSON.stringify) without actually testing the cast.

Comment thread apps/sim/lib/table/sql.ts
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit baf6585. Configure here.

Comment thread apps/sim/lib/table/service.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant